Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Sep 3, 2025

Description

This PR fixes an issue where the countdown timer would incorrectly reappear when viewing task history, even after a follow-up suggestion had been selected.

Problem

When users:

  1. Enable Auto-Approve 'Question'
  2. Ask Roo a question and receive suggestions
  3. Select a suggestion (not the first one) before the countdown ends
  4. View the task history later

The countdown timer would incorrectly show again in the history view, even though the suggestion had already been selected.

Solution

The fix adds logic to properly determine if a follow-up question has been answered:

  • For the current session: Uses the existing currentFollowUpTs check
  • For historical messages: Checks if there are any messages after the follow-up question (indicating it was answered)

Testing

  • ✅ All existing tests pass
  • ✅ Type checking passes
  • ✅ Linting passes
  • ✅ Manual testing confirms the countdown no longer appears in history for answered questions

Related Issue

Fixes #7624

Review Confidence

The implementation review showed 95% confidence with no security issues or code quality concerns.


Important

Fixes countdown timer reappearing in task history for answered follow-up questions by adding logic in ChatView.tsx.

  • Behavior:
    • Fixes issue where countdown timer reappears in task history for answered follow-up questions.
    • Adds logic in ChatView.tsx to check if a follow-up question is answered by verifying currentFollowUpTs for current sessions and checking subsequent messages for historical ones.
  • Testing:
    • All existing tests, type checking, and linting pass.
    • Manual testing confirms the fix.
  • Misc:

This description was created by Ellipsis for 4fc9982. You can customize this summary. It will automatically update as commits are pushed.

…low-up questions

- Check if follow-up question has been answered by looking for subsequent messages
- For current session, use existing currentFollowUpTs check
- For history, check if there are messages after the follow-up question
- Fixes #7624
@roomote roomote bot requested review from cte, jr and mrubens as code owners September 3, 2025 09:57
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Sep 3, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this code 2 minutes ago and already found 5 ways I could've done it better.

// Check if this follow-up question has been answered
// For current session: check if it matches currentFollowUpTs
// For history: check if there's a message after this follow-up question
const isFollowUpAnswered = (() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice this logic is duplicated from what appears around line 1521. Could we extract this into a reusable function to follow DRY principles? Something like:

}
// For historical messages, check if there's a message after this follow-up
// If there is, it means the follow-up was answered
const messageIndex = modifiedMessages.findIndex((msg) => msg.ts === messageOrGroup.ts)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using on for every render could impact performance with large message lists. Have you considered memoizing this check or perhaps storing the answered state in a more efficient data structure?

// If there is, it means the follow-up was answered
const messageIndex = modifiedMessages.findIndex((msg) => msg.ts === messageOrGroup.ts)
if (messageIndex !== -1 && messageIndex < modifiedMessages.length - 1) {
// There's at least one message after this follow-up, so it was answered
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current logic assumes any message after a follow-up means it was answered. What if there's an error message or system message immediately after? Should we verify the next message is actually a user response or an action that indicates the follow-up was handled?


// regular message
// Check if this follow-up question has been answered
// For current session: check if it matches currentFollowUpTs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments are helpful but could be more concise. Consider combining them:

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 3, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 4, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 4, 2025
@daniel-lxs daniel-lxs closed this Sep 5, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 5, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Countdown still appears when suggestion selected

4 participants